Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

t3c parentdotconfig: ensure ocgmap is ordered by prim, sec #7411

Merged
merged 3 commits into from
Mar 27, 2023

Conversation

traeak
Copy link
Contributor

@traeak traeak commented Mar 18, 2023

A bug in t3c was found when a topology for a MID is simulated for a non topology MSO delivery service with 2 origins in the primary and secondary groups and round_robin=false. The primary and secondary origin parent cache groups assignments weren't properly being transferred over to the created topology resulting in inconsistent origin ordering in parent.config

This bug fixes that by explicitly assigning primary and secondary parent cache groups to the intermediate struct.


Which Traffic Control components are affected by this PR?

  • Traffic Control Cache Config (t3c, formerly ORT)

What is the best way to verify this PR?

Tested by creating a DS with:

  • Not consistent hash (round_robin=false)
  • Mid cache group with primary and secondary parent
  • Origin assigned from one of each of the above cache groups.

Run t3c multiple times and ensure the parent.config is consistently created.

If this is a bugfix, which Traffic Control versions contained the bug?

  • 7.1.0

PR submission checklist

@codecov
Copy link

codecov bot commented Mar 18, 2023

Codecov Report

Merging #7411 (20e1cae) into master (f7765ff) will increase coverage by 0.01%.
The diff coverage is 85.00%.

@@             Coverage Diff              @@
##             master    #7411      +/-   ##
============================================
+ Coverage     27.03%   27.04%   +0.01%     
  Complexity       98       98              
============================================
  Files           685      685              
  Lines         77279    77301      +22     
  Branches         90       90              
============================================
+ Hits          20892    20909      +17     
- Misses        54399    54404       +5     
  Partials       1988     1988              
Flag Coverage Δ
golib_unit 48.40% <85.00%> (+0.05%) ⬆️
grove_unit 4.60% <ø> (ø)
t3c_unit 5.32% <ø> (ø)
traffic_monitor_unit 20.43% <ø> (ø)
traffic_ops_unit 22.19% <ø> (ø)
traffic_stats_unit 10.14% <ø> (ø)
unit_tests 23.60% <85.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/go-atscfg/parentdotconfig.go 65.22% <85.00%> (+0.29%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@traeak traeak force-pushed the fix_nontopo_mso_parent branch 2 times, most recently from 9c7ed6c to 2ddfcb8 Compare March 18, 2023 15:01
@ocket8888 ocket8888 added bug something isn't working as intended low impact affects only a small portion of a CDN, and cannot itself break one cache-config Cache config generation labels Mar 20, 2023
Copy link
Contributor

@jpappa200 jpappa200 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected. consistent ordering of primary and secondary origins. t3c integration tests failed, but they may just need to be started again. +1 once checks are successful and conflicts are resolved.

@ocket8888 ocket8888 merged commit d257cb5 into apache:master Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something isn't working as intended cache-config Cache config generation low impact affects only a small portion of a CDN, and cannot itself break one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants